Create New SSZ Factory for Arrays of Roots #91
Conversation
Codecov Report
@@ Coverage Diff @@
## master #91 +/- ##
==========================================
- Coverage 78.99% 75.39% -3.61%
==========================================
Files 12 13 +1
Lines 1157 1272 +115
==========================================
+ Hits 914 959 +45
- Misses 138 190 +52
- Partials 105 123 +18
Continue to review full report at Codecov.
|
rauljordan
changed the title
rauljordan
force-pushed
the
eth2-experiment
branch
from
bc1973f
to
a9781b3
Compare
rauljordan
changed the title
rauljordan
requested review from
0xKiwi,
nisdas,
prestonvanloon,
shayzluf and
terencechain
| @@ -25,31 +25,18 @@ func newBasicArraySSZ() *basicArraySSZ { | |||
| } | |||
| } | |||
|
|
|||
| func (b *basicArraySSZ) Root(val reflect.Value, typ reflect.Type, maxCapacity uint64) ([32]byte, error) { | |||
| func (b *basicArraySSZ) Root(val reflect.Value, typ reflect.Type, fieldName string, maxCapacity uint64) ([32]byte, error) { | |||
There was a problem hiding this comment.
why do you need fieldName ? it isnt used in basicArraySSZ
There was a problem hiding this comment.
I had to add it everywhere because all of these functions need to meet the same interface, and it could be useful down the line for further caching or namespacing
| var enableCache = false | ||
|
|
||
| // ToggleCache enables caching of ssz hash tree root. It is disabled by default. | ||
| func ToggleCache(val bool) { |
There was a problem hiding this comment.
this is only for testing ,correct ?
There was a problem hiding this comment.
Hmm to some extent - it is nice to have the functionality of disabling caching though, either for benchmark reasons or for other reasons. It is the best compromise I found to prevent collision resistance in tests and maintain speed.
There was a problem hiding this comment.
sounds good. Would be an easy way to debug the cache if we have anything wrong
No tracking issue.
Background
This PR works on optimizing one of our biggest bottlenecks in Prysm, which is the usage of hash tree root on basic arrays of roots. If a single element changes, we have to recompute the entire function again, making the processing of slots in the beacon state extremely slow when called repeatedly.
Changes
This PR introduces the following changes:
ToggleCachepublic function that allows for disabling the cache when running ssz related items in spec tests for preventing collisions